Skip to content

Streamline config in bootstrap #144807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

Shourya742
Copy link
Contributor

This PR restructures the config module to improve readability and debuggability. It also aims to eliminate as many invariants as possible. Best reviewed commit by commit.

r? @Kobzol

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Aug 2, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 2, 2025

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

@Shourya742 Shourya742 marked this pull request as draft August 2, 2025 06:14
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2025
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would stop before eb81f72 in this PR, it already has a lot of changes. Left some comments. Looks great!

@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-07-30-streamline-config branch from 87966f4 to cf03a40 Compare August 4, 2025 15:19
@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-07-30-streamline-config branch from cf03a40 to b32ded3 Compare August 4, 2025 16:07
@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-07-30-streamline-config branch from b32ded3 to 888edc0 Compare August 4, 2025 16:40
@rust-log-analyzer

This comment has been minimized.

@Shourya742 Shourya742 force-pushed the 2025-07-30-streamline-config branch from 888edc0 to a2794f9 Compare August 4, 2025 18:38
@Shourya742 Shourya742 marked this pull request as ready for review August 5, 2025 05:45
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 5, 2025
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, GH doesn't send notifications for PRs being marked as ready to review, annoying as always. Found one thing.

@@ -977,11 +1163,90 @@ impl Config {
config.channel = channel;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This override is applied after config.channel has already been read.. but it seems to be pre-existing, not sure if it was intended though. Let's leave it like it is for now.

@Shourya742 Shourya742 force-pushed the 2025-07-30-streamline-config branch from a2794f9 to 962836d Compare August 6, 2025 10:21
@Kobzol
Copy link
Member

Kobzol commented Aug 6, 2025

Thanks, looks good now. You can r=me once CI is green.

@bors delegate+

@bors
Copy link
Collaborator

bors commented Aug 6, 2025

✌️ @Shourya742, you can now approve this pull request!

If @Kobzol told you to "r=me" after making some further change, please make that change, then do @bors r=@Kobzol

@Shourya742
Copy link
Contributor Author

@bors r=@Kobzol

@bors
Copy link
Collaborator

bors commented Aug 6, 2025

📌 Commit 962836d has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants